Skip to content

libnvmf: replace nvmf_get_discovery_log() with opaque-args API#3259

Merged
igaw merged 4 commits intolinux-nvme:masterfrom
martin-belanger:discovery-api-refactor
Apr 10, 2026
Merged

libnvmf: replace nvmf_get_discovery_log() with opaque-args API#3259
igaw merged 4 commits intolinux-nvme:masterfrom
martin-belanger:discovery-api-refactor

Conversation

@martin-belanger
Copy link
Copy Markdown

@martin-belanger martin-belanger commented Apr 9, 2026

This PR contains 4 commits: 3 preparatory bug-fix / clean-up commits followed by the actual refactor.

Commit 1 – bug fix: nvmf: fix timeout not applied to discovery log commands
The args->timeout value was never forwarded to cmd.timeout_ms in nvme_discovery_log(), so all three Get Log Page commands always used the kernel default timeout.

Commit 2 – bug fix: nvmf: fix generator emitting setters for read-only members in .ld files
The generator's generate_ld() was unconditionally emitting both getter and setter entries for every struct member, ignoring the is_const flag that correctly suppresses setters in the .h/.c output. Regenerate accessors.ld to drop the spurious setter entries.

Commit 3 – clean-up: nvmf: clean up discovery args struct initialization
Remove redundant zero-initializers (.result = 0) and replace the magic literal 0 for lsp with the named constant NVMF_LOG_DISC_LSP_NONE.

Commit 4 – refactor: nvmf: replace nvmf_get_discovery_log() with opaque-args API
Introduce struct nvmf_discovery_args as an ABI-stable opaque object with auto-generated setter/getter accessors. The mandatory controller argument moves to the function signature; optional parameters (timeout, max_retries, lsp) are set via accessors. Callers may pass NULL for args to get built-in defaults. Remove the old nvmf_get_discovery_wargs() API entirely.

Split the accessor generator output into a common NVMe triplet (accessors.{h,c,ld}) and an NVMe-oF-specific triplet (nvmf-accessors.{h,c,ld}) so that non-fabrics builds can exclude all fabrics code. The generator script is made generic; two meson targets (update-common-accessors, update-fabrics-accessors) are exposed under the update-accessors alias.

@martin-belanger martin-belanger force-pushed the discovery-api-refactor branch 2 times, most recently from fbc9c52 to c53a114 Compare April 9, 2026 17:06
Martin Belanger added 3 commits April 9, 2026 13:18
args->timeout was set in the discovery args struct but never copied
into cmd.timeout_ms before issuing the log page commands, so the
caller-specified timeout had no effect.

Signed-off-by: Martin Belanger <[email protected]>
generate_ld() always emitted both getter and setter entries regardless
of the member's is_const flag, unlike generate_hdr() and generate_src()
which both gate the setter on not is_const. As a result, accessors.ld
exported setter symbols that had no __public definition in accessors.c.

Also introduce LD_BANNER (a compact copyright block without the ASCII
art) for generated .ld files, since the "Generated Code" art is only
useful in .h/.c files that editors open regularly.

Regenerate accessors.ld to remove the spurious setter entries and
update the banner.

Signed-off-by: Martin Belanger <[email protected]>
Replace literal zero initializers with the proper named constants:
  .result = 0  -> removed (zero is the default for unspecified fields)
  .lsp = 0     -> .lsp = NVMF_LOG_DISC_LSP_NONE

Also add the missing .timeout = NVME_DEFAULT_IOCTL_TIMEOUT in
_nvmf_discovery() to be consistent with nbft_discovery().

Signed-off-by: Martin Belanger <[email protected]>
@martin-belanger martin-belanger force-pushed the discovery-api-refactor branch 3 times, most recently from d696b93 to 12395b1 Compare April 9, 2026 18:04
Comment thread libnvme/examples/discover-loop.c Outdated
Comment thread libnvme/src/nvme/fabrics.c Outdated
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Apr 10, 2026

Looks good. Just the one nitpick and as I said, I would drop the timeout value for now. There is little point explicitly setting the default value '0' which means the kernel is using its defaults. If there are commands which need a special treatment we can add it without a problem for this specific case.

@martin-belanger martin-belanger force-pushed the discovery-api-refactor branch from 12395b1 to 32b5cae Compare April 10, 2026 10:24
Introduce struct nvmf_discovery_args as an ABI-stable opaque object
with auto-generated setter/getter accessors (nvmf-accessors.{h,c,ld}).
The mandatory controller argument moves to the function signature;
optional parameters (timeout, max_retries, lsp) are set via accessors.
Remove nvmf_get_discovery_wargs() entirely.

Split the accessor generator output into a common NVMe triplet
(accessors.{h,c,ld}) and an NVMe-oF-specific triplet
(nvmf-accessors.{h,c,ld}) so that PCIe/embedded builds can exclude
all fabrics code. The generator script is made generic; two meson
targets (update-common-accessors, update-fabrics-accessors) are
exposed under the update-accessors alias.

Signed-off-by: Martin Belanger <[email protected]>
@martin-belanger martin-belanger force-pushed the discovery-api-refactor branch from 32b5cae to f55f187 Compare April 10, 2026 10:39
@igaw igaw merged commit 4edcf83 into linux-nvme:master Apr 10, 2026
28 of 29 checks passed
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Apr 10, 2026

Thanks!

@martin-belanger martin-belanger deleted the discovery-api-refactor branch April 10, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants